-
-
Notifications
You must be signed in to change notification settings - Fork 639
fix: Add linux_riscv64 to _pip_repository_impl #3350
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Summary of ChangesHello @kxxt, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request correctly adds linux_riscv64 to the list of supported platforms in _pip_repository_impl, which is a good first step towards supporting this architecture. However, the fix appears to be incomplete. For full support, especially for parsing wheel filenames, the wheel platform tag parser also needs to be updated to recognize the riscv64 architecture. I've left a detailed comment on the .bzl file change with guidance on how to complete the implementation.
f108a9b to
8c0d41d
Compare
CI fails with connection timed out
|
It appears that |
|
FYI @meteorcloudy, @rickeylev, recently the macos build has been flaking a lot download errors like this.
|
Thanks for the heads-up. I tested this PR by converting it to a patch based on rules_python 1.6.0 to use in tensorflow's |
|
Yes, sorry we are having some CI outage for macOS workers: bazelbuild/continuous-integration#2385 |
Hi, @aignas. I tested this PR again by bumping tensorflow's rules_python to The build of tensorflow fails but I think that's because of other changes in |
manylinux_X_Y_* is covered by manylinux_11_12_riscv64 pattern where _11_ and _12_ will be replaced.
Imported from GitHub PR tensorflow/tensorflow#102607 This commit fixes #102159 which prevented TensorFlow from compiling on RISC-V 64 due to missing codepath in the build system. RFC: Currently this patch overrides timeout of rules_python for all platforms which is obviously not ideal. How do we set pip package timeout here in Tensorflow? See also: - bazelbuild/bazel#25236 - bazelbuild/bazel#25699 - bazel-contrib/rules_python#3350 - #32812 - tensorflow/runtime#135 Copybara import of the project: -- 13b7db883e6ea0aa631490fb5f806c55ceea7cb5 by gns <[email protected]>: [XLA:CPU] Add support for riscv64 Co-authored-by: Levi Zim <[email protected]> -- a00c3a3ba94664798960cbc2fb0a6746fe0c3ac1 by gns <[email protected]>: Add missing llvm:: namespace qualifiers for DTensor Add the `llvm::` namespace prefix to `cast` and `isa` where it was missing in the DTensor MLIR code. -- febc5abc56ab302f5f39fe20176ed00c0c95cb7e by gns <[email protected]>: runtime: add missing stdint header Co-authored-by: Levi Zim <[email protected]> -- 6bf63fec56a6d961b06d932fe69b76eae89d8cb7 by gns <[email protected]>: Add riscv64 support patch for rules_python bazel-contrib/rules_python#3350 -- 68c44fa9613d5577f36c51a192c932795e13ed83 by gns <[email protected]>: tools: py: add riscv64 to pip and manylinux compilance test -- 3f2fe1e0256476449f64658219d997727c9cc534 by gns <[email protected]>: Lift timeout of rules_python for BFS wheels -- 252a9b7ad2336b7abbb94e269b4a976b45d5d6e7 by gns <[email protected]>: Revert llvm toolchain patch for riscv64 As already upstreamed. Co-authored-by: Levi Zim <[email protected]> -- 51047a86131e991ffea74fccb8f55e3521237a64 by gns <[email protected]>: Lift timeout for BFS wheels during pip init -- 3b8276420031969423c9a1f150341a0e41fa372e by gns <[email protected]>: Refresh `rules_python` riscv64 patch Co-authored-by: Levi Zim <[email protected]> Merging this change closes #102607 FUTURE_COPYBARA_INTEGRATE_REVIEW=tensorflow/tensorflow#102607 from infiWang:riscv64 3b8276420031969423c9a1f150341a0e41fa372e PiperOrigin-RevId: 825073762
Imported from GitHub PR tensorflow/tensorflow#102607 This commit fixes #102159 which prevented TensorFlow from compiling on RISC-V 64 due to missing codepath in the build system. RFC: Currently this patch overrides timeout of rules_python for all platforms which is obviously not ideal. How do we set pip package timeout here in Tensorflow? See also: - bazelbuild/bazel#25236 - bazelbuild/bazel#25699 - bazel-contrib/rules_python#3350 - #32812 - tensorflow/runtime#135 Copybara import of the project: -- 13b7db883e6ea0aa631490fb5f806c55ceea7cb5 by gns <[email protected]>: [XLA:CPU] Add support for riscv64 Co-authored-by: Levi Zim <[email protected]> -- a00c3a3ba94664798960cbc2fb0a6746fe0c3ac1 by gns <[email protected]>: Add missing llvm:: namespace qualifiers for DTensor Add the `llvm::` namespace prefix to `cast` and `isa` where it was missing in the DTensor MLIR code. -- febc5abc56ab302f5f39fe20176ed00c0c95cb7e by gns <[email protected]>: runtime: add missing stdint header Co-authored-by: Levi Zim <[email protected]> -- 6bf63fec56a6d961b06d932fe69b76eae89d8cb7 by gns <[email protected]>: Add riscv64 support patch for rules_python bazel-contrib/rules_python#3350 -- 68c44fa9613d5577f36c51a192c932795e13ed83 by gns <[email protected]>: tools: py: add riscv64 to pip and manylinux compilance test -- 3f2fe1e0256476449f64658219d997727c9cc534 by gns <[email protected]>: Lift timeout of rules_python for BFS wheels -- 252a9b7ad2336b7abbb94e269b4a976b45d5d6e7 by gns <[email protected]>: Revert llvm toolchain patch for riscv64 As already upstreamed. Co-authored-by: Levi Zim <[email protected]> -- 51047a86131e991ffea74fccb8f55e3521237a64 by gns <[email protected]>: Lift timeout for BFS wheels during pip init -- 3b8276420031969423c9a1f150341a0e41fa372e by gns <[email protected]>: Refresh `rules_python` riscv64 patch Co-authored-by: Levi Zim <[email protected]> Merging this change closes #102607 FUTURE_COPYBARA_INTEGRATE_REVIEW=tensorflow/tensorflow#102607 from infiWang:riscv64 3b8276420031969423c9a1f150341a0e41fa372e PiperOrigin-RevId: 825073762
Imported from GitHub PR tensorflow/tensorflow#102607 This commit fixes #102159 which prevented TensorFlow from compiling on RISC-V 64 due to missing codepath in the build system. RFC: Currently this patch overrides timeout of rules_python for all platforms which is obviously not ideal. How do we set pip package timeout here in Tensorflow? See also: - bazelbuild/bazel#25236 - bazelbuild/bazel#25699 - bazel-contrib/rules_python#3350 - #32812 - tensorflow/runtime#135 Copybara import of the project: -- 13b7db883e6ea0aa631490fb5f806c55ceea7cb5 by gns <[email protected]>: [XLA:CPU] Add support for riscv64 Co-authored-by: Levi Zim <[email protected]> -- a00c3a3ba94664798960cbc2fb0a6746fe0c3ac1 by gns <[email protected]>: Add missing llvm:: namespace qualifiers for DTensor Add the `llvm::` namespace prefix to `cast` and `isa` where it was missing in the DTensor MLIR code. -- febc5abc56ab302f5f39fe20176ed00c0c95cb7e by gns <[email protected]>: runtime: add missing stdint header Co-authored-by: Levi Zim <[email protected]> -- 6bf63fec56a6d961b06d932fe69b76eae89d8cb7 by gns <[email protected]>: Add riscv64 support patch for rules_python bazel-contrib/rules_python#3350 -- 68c44fa9613d5577f36c51a192c932795e13ed83 by gns <[email protected]>: tools: py: add riscv64 to pip and manylinux compilance test -- 3f2fe1e0256476449f64658219d997727c9cc534 by gns <[email protected]>: Lift timeout of rules_python for BFS wheels -- 252a9b7ad2336b7abbb94e269b4a976b45d5d6e7 by gns <[email protected]>: Revert llvm toolchain patch for riscv64 As already upstreamed. Co-authored-by: Levi Zim <[email protected]> -- 51047a86131e991ffea74fccb8f55e3521237a64 by gns <[email protected]>: Lift timeout for BFS wheels during pip init -- 3b8276420031969423c9a1f150341a0e41fa372e by gns <[email protected]>: Refresh `rules_python` riscv64 patch Co-authored-by: Levi Zim <[email protected]> Merging this change closes #102607 FUTURE_COPYBARA_INTEGRATE_REVIEW=tensorflow/tensorflow#102607 from infiWang:riscv64 3b8276420031969423c9a1f150341a0e41fa372e PiperOrigin-RevId: 825073762
Imported from GitHub PR tensorflow/tensorflow#102607 This commit fixes #102159 which prevented TensorFlow from compiling on RISC-V 64 due to missing codepath in the build system. RFC: Currently this patch overrides timeout of rules_python for all platforms which is obviously not ideal. How do we set pip package timeout here in Tensorflow? See also: - bazelbuild/bazel#25236 - bazelbuild/bazel#25699 - bazel-contrib/rules_python#3350 - #32812 - tensorflow/runtime#135 Copybara import of the project: -- 13b7db883e6ea0aa631490fb5f806c55ceea7cb5 by gns <[email protected]>: [XLA:CPU] Add support for riscv64 Co-authored-by: Levi Zim <[email protected]> -- a00c3a3ba94664798960cbc2fb0a6746fe0c3ac1 by gns <[email protected]>: Add missing llvm:: namespace qualifiers for DTensor Add the `llvm::` namespace prefix to `cast` and `isa` where it was missing in the DTensor MLIR code. -- febc5abc56ab302f5f39fe20176ed00c0c95cb7e by gns <[email protected]>: runtime: add missing stdint header Co-authored-by: Levi Zim <[email protected]> -- 6bf63fec56a6d961b06d932fe69b76eae89d8cb7 by gns <[email protected]>: Add riscv64 support patch for rules_python bazel-contrib/rules_python#3350 -- 68c44fa9613d5577f36c51a192c932795e13ed83 by gns <[email protected]>: tools: py: add riscv64 to pip and manylinux compilance test -- 3f2fe1e0256476449f64658219d997727c9cc534 by gns <[email protected]>: Lift timeout of rules_python for BFS wheels -- 252a9b7ad2336b7abbb94e269b4a976b45d5d6e7 by gns <[email protected]>: Revert llvm toolchain patch for riscv64 As already upstreamed. Co-authored-by: Levi Zim <[email protected]> -- 51047a86131e991ffea74fccb8f55e3521237a64 by gns <[email protected]>: Lift timeout for BFS wheels during pip init -- 3b8276420031969423c9a1f150341a0e41fa372e by gns <[email protected]>: Refresh `rules_python` riscv64 patch Co-authored-by: Levi Zim <[email protected]> Merging this change closes #102607 FUTURE_COPYBARA_INTEGRATE_REVIEW=tensorflow/tensorflow#102607 from infiWang:riscv64 3b8276420031969423c9a1f150341a0e41fa372e PiperOrigin-RevId: 825073762
Imported from GitHub PR tensorflow/tensorflow#102607 This commit fixes #102159 which prevented TensorFlow from compiling on RISC-V 64 due to missing codepath in the build system. RFC: Currently this patch overrides timeout of rules_python for all platforms which is obviously not ideal. How do we set pip package timeout here in Tensorflow? See also: - bazelbuild/bazel#25236 - bazelbuild/bazel#25699 - bazel-contrib/rules_python#3350 - #32812 - tensorflow/runtime#135 Copybara import of the project: -- 13b7db883e6ea0aa631490fb5f806c55ceea7cb5 by gns <[email protected]>: [XLA:CPU] Add support for riscv64 Co-authored-by: Levi Zim <[email protected]> -- a00c3a3ba94664798960cbc2fb0a6746fe0c3ac1 by gns <[email protected]>: Add missing llvm:: namespace qualifiers for DTensor Add the `llvm::` namespace prefix to `cast` and `isa` where it was missing in the DTensor MLIR code. -- febc5abc56ab302f5f39fe20176ed00c0c95cb7e by gns <[email protected]>: runtime: add missing stdint header Co-authored-by: Levi Zim <[email protected]> -- 6bf63fec56a6d961b06d932fe69b76eae89d8cb7 by gns <[email protected]>: Add riscv64 support patch for rules_python bazel-contrib/rules_python#3350 -- 68c44fa9613d5577f36c51a192c932795e13ed83 by gns <[email protected]>: tools: py: add riscv64 to pip and manylinux compilance test -- 3f2fe1e0256476449f64658219d997727c9cc534 by gns <[email protected]>: Lift timeout of rules_python for BFS wheels -- 252a9b7ad2336b7abbb94e269b4a976b45d5d6e7 by gns <[email protected]>: Revert llvm toolchain patch for riscv64 As already upstreamed. Co-authored-by: Levi Zim <[email protected]> -- 51047a86131e991ffea74fccb8f55e3521237a64 by gns <[email protected]>: Lift timeout for BFS wheels during pip init -- 3b8276420031969423c9a1f150341a0e41fa372e by gns <[email protected]>: Refresh `rules_python` riscv64 patch Co-authored-by: Levi Zim <[email protected]> Merging this change closes #102607 FUTURE_COPYBARA_INTEGRATE_REVIEW=tensorflow/tensorflow#102607 from infiWang:riscv64 3b8276420031969423c9a1f150341a0e41fa372e PiperOrigin-RevId: 825073762
Imported from GitHub PR tensorflow/tensorflow#102607 This commit fixes #102159 which prevented TensorFlow from compiling on RISC-V 64 due to missing codepath in the build system. RFC: Currently this patch overrides timeout of rules_python for all platforms which is obviously not ideal. How do we set pip package timeout here in Tensorflow? See also: - bazelbuild/bazel#25236 - bazelbuild/bazel#25699 - bazel-contrib/rules_python#3350 - #32812 - tensorflow/runtime#135 Copybara import of the project: -- 13b7db883e6ea0aa631490fb5f806c55ceea7cb5 by gns <[email protected]>: [XLA:CPU] Add support for riscv64 Co-authored-by: Levi Zim <[email protected]> -- a00c3a3ba94664798960cbc2fb0a6746fe0c3ac1 by gns <[email protected]>: Add missing llvm:: namespace qualifiers for DTensor Add the `llvm::` namespace prefix to `cast` and `isa` where it was missing in the DTensor MLIR code. -- febc5abc56ab302f5f39fe20176ed00c0c95cb7e by gns <[email protected]>: runtime: add missing stdint header Co-authored-by: Levi Zim <[email protected]> -- 6bf63fec56a6d961b06d932fe69b76eae89d8cb7 by gns <[email protected]>: Add riscv64 support patch for rules_python bazel-contrib/rules_python#3350 -- 68c44fa9613d5577f36c51a192c932795e13ed83 by gns <[email protected]>: tools: py: add riscv64 to pip and manylinux compilance test -- 3f2fe1e0256476449f64658219d997727c9cc534 by gns <[email protected]>: Lift timeout of rules_python for BFS wheels -- 252a9b7ad2336b7abbb94e269b4a976b45d5d6e7 by gns <[email protected]>: Revert llvm toolchain patch for riscv64 As already upstreamed. Co-authored-by: Levi Zim <[email protected]> -- 51047a86131e991ffea74fccb8f55e3521237a64 by gns <[email protected]>: Lift timeout for BFS wheels during pip init -- 3b8276420031969423c9a1f150341a0e41fa372e by gns <[email protected]>: Refresh `rules_python` riscv64 patch Co-authored-by: Levi Zim <[email protected]> Merging this change closes #102607 FUTURE_COPYBARA_INTEGRATE_REVIEW=tensorflow/tensorflow#102607 from infiWang:riscv64 3b8276420031969423c9a1f150341a0e41fa372e PiperOrigin-RevId: 825073762
Imported from GitHub PR tensorflow/tensorflow#102607 This commit fixes #102159 which prevented TensorFlow from compiling on RISC-V 64 due to missing codepath in the build system. RFC: Currently this patch overrides timeout of rules_python for all platforms which is obviously not ideal. How do we set pip package timeout here in Tensorflow? See also: - bazelbuild/bazel#25236 - bazelbuild/bazel#25699 - bazel-contrib/rules_python#3350 - #32812 - tensorflow/runtime#135 Copybara import of the project: -- 13b7db883e6ea0aa631490fb5f806c55ceea7cb5 by gns <[email protected]>: [XLA:CPU] Add support for riscv64 Co-authored-by: Levi Zim <[email protected]> -- a00c3a3ba94664798960cbc2fb0a6746fe0c3ac1 by gns <[email protected]>: Add missing llvm:: namespace qualifiers for DTensor Add the `llvm::` namespace prefix to `cast` and `isa` where it was missing in the DTensor MLIR code. -- febc5abc56ab302f5f39fe20176ed00c0c95cb7e by gns <[email protected]>: runtime: add missing stdint header Co-authored-by: Levi Zim <[email protected]> -- 6bf63fec56a6d961b06d932fe69b76eae89d8cb7 by gns <[email protected]>: Add riscv64 support patch for rules_python bazel-contrib/rules_python#3350 -- 68c44fa9613d5577f36c51a192c932795e13ed83 by gns <[email protected]>: tools: py: add riscv64 to pip and manylinux compilance test -- 3f2fe1e0256476449f64658219d997727c9cc534 by gns <[email protected]>: Lift timeout of rules_python for BFS wheels -- 252a9b7ad2336b7abbb94e269b4a976b45d5d6e7 by gns <[email protected]>: Revert llvm toolchain patch for riscv64 As already upstreamed. Co-authored-by: Levi Zim <[email protected]> -- 51047a86131e991ffea74fccb8f55e3521237a64 by gns <[email protected]>: Lift timeout for BFS wheels during pip init -- 3b8276420031969423c9a1f150341a0e41fa372e by gns <[email protected]>: Refresh `rules_python` riscv64 patch Co-authored-by: Levi Zim <[email protected]> Merging this change closes #102607 FUTURE_COPYBARA_INTEGRATE_REVIEW=tensorflow/tensorflow#102607 from infiWang:riscv64 3b8276420031969423c9a1f150341a0e41fa372e PiperOrigin-RevId: 825073762
Imported from GitHub PR tensorflow/tensorflow#102607 This commit fixes #102159 which prevented TensorFlow from compiling on RISC-V 64 due to missing codepath in the build system. RFC: Currently this patch overrides timeout of rules_python for all platforms which is obviously not ideal. How do we set pip package timeout here in Tensorflow? See also: - bazelbuild/bazel#25236 - bazelbuild/bazel#25699 - bazel-contrib/rules_python#3350 - #32812 - tensorflow/runtime#135 Copybara import of the project: -- 13b7db883e6ea0aa631490fb5f806c55ceea7cb5 by gns <[email protected]>: [XLA:CPU] Add support for riscv64 Co-authored-by: Levi Zim <[email protected]> -- a00c3a3ba94664798960cbc2fb0a6746fe0c3ac1 by gns <[email protected]>: Add missing llvm:: namespace qualifiers for DTensor Add the `llvm::` namespace prefix to `cast` and `isa` where it was missing in the DTensor MLIR code. -- febc5abc56ab302f5f39fe20176ed00c0c95cb7e by gns <[email protected]>: runtime: add missing stdint header Co-authored-by: Levi Zim <[email protected]> -- 6bf63fec56a6d961b06d932fe69b76eae89d8cb7 by gns <[email protected]>: Add riscv64 support patch for rules_python bazel-contrib/rules_python#3350 -- 68c44fa9613d5577f36c51a192c932795e13ed83 by gns <[email protected]>: tools: py: add riscv64 to pip and manylinux compilance test -- 3f2fe1e0256476449f64658219d997727c9cc534 by gns <[email protected]>: Lift timeout of rules_python for BFS wheels -- 252a9b7ad2336b7abbb94e269b4a976b45d5d6e7 by gns <[email protected]>: Revert llvm toolchain patch for riscv64 As already upstreamed. Co-authored-by: Levi Zim <[email protected]> -- 51047a86131e991ffea74fccb8f55e3521237a64 by gns <[email protected]>: Lift timeout for BFS wheels during pip init -- 3b8276420031969423c9a1f150341a0e41fa372e by gns <[email protected]>: Refresh `rules_python` riscv64 patch Co-authored-by: Levi Zim <[email protected]> Merging this change closes #102607 FUTURE_COPYBARA_INTEGRATE_REVIEW=tensorflow/tensorflow#102607 from infiWang:riscv64 3b8276420031969423c9a1f150341a0e41fa372e PiperOrigin-RevId: 825073762
Imported from GitHub PR tensorflow/tensorflow#102607 This commit fixes #102159 which prevented TensorFlow from compiling on RISC-V 64 due to missing codepath in the build system. RFC: Currently this patch overrides timeout of rules_python for all platforms which is obviously not ideal. How do we set pip package timeout here in Tensorflow? See also: - bazelbuild/bazel#25236 - bazelbuild/bazel#25699 - bazel-contrib/rules_python#3350 - #32812 - tensorflow/runtime#135 Copybara import of the project: -- 13b7db883e6ea0aa631490fb5f806c55ceea7cb5 by gns <[email protected]>: [XLA:CPU] Add support for riscv64 Co-authored-by: Levi Zim <[email protected]> -- a00c3a3ba94664798960cbc2fb0a6746fe0c3ac1 by gns <[email protected]>: Add missing llvm:: namespace qualifiers for DTensor Add the `llvm::` namespace prefix to `cast` and `isa` where it was missing in the DTensor MLIR code. -- febc5abc56ab302f5f39fe20176ed00c0c95cb7e by gns <[email protected]>: runtime: add missing stdint header Co-authored-by: Levi Zim <[email protected]> -- 6bf63fec56a6d961b06d932fe69b76eae89d8cb7 by gns <[email protected]>: Add riscv64 support patch for rules_python bazel-contrib/rules_python#3350 -- 68c44fa9613d5577f36c51a192c932795e13ed83 by gns <[email protected]>: tools: py: add riscv64 to pip and manylinux compilance test -- 3f2fe1e0256476449f64658219d997727c9cc534 by gns <[email protected]>: Lift timeout of rules_python for BFS wheels -- 252a9b7ad2336b7abbb94e269b4a976b45d5d6e7 by gns <[email protected]>: Revert llvm toolchain patch for riscv64 As already upstreamed. Co-authored-by: Levi Zim <[email protected]> -- 51047a86131e991ffea74fccb8f55e3521237a64 by gns <[email protected]>: Lift timeout for BFS wheels during pip init -- 3b8276420031969423c9a1f150341a0e41fa372e by gns <[email protected]>: Refresh `rules_python` riscv64 patch Co-authored-by: Levi Zim <[email protected]> Merging this change closes #102607 FUTURE_COPYBARA_INTEGRATE_REVIEW=tensorflow/tensorflow#102607 from infiWang:riscv64 3b8276420031969423c9a1f150341a0e41fa372e PiperOrigin-RevId: 825073762
Add
linux_riscv64support for pulling pip dependencies. This is notadding any hermetic toolchain support - user has to provide a working
toolchain.
Fix #2729